ERA-11293 [Part 3: React 19] #1280
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades React to version 19 by removing PropTypes and ref forwarding in several components as part of the migration away from CRA and Webpack. Key changes include:
- Removal of forwardRef and associated eslint disable comments in components such as Input and MenuPopover.
- Elimination of PropTypes, defaultProps, and propTypes definitions throughout multiple files.
- Minor adjustments to default prop assignments and destructuring across components.
Reviewed Changes
Copilot reviewed 156 out of 157 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DatePicker/CalendarPopper/MonthPicker/index.js | Removed forwardRef from the Input component and updated prop handling. |
| src/DataExportModal/index.js | Removed PropTypes definitions and set default prop values inline. |
| src/DasIcon/index.js | Removed PropTypes definitions from the component. |
| src/CursorGpsDisplay/MenuPopover/index.js | Removed forwardRef wrapping and updated the component’s prop handling. |
| src/ContextMenu/index.js | Removed PropTypes definitions and relied on default parameter values. |
| src/ColumnSort/index.js | Removed PropTypes and updated default prop values. |
| src/ClustersLayer/index.js | Removed PropTypes from the component. |
| src/Checkmark/index.js | Removed defaultProps and PropTypes for CheckMark. |
| src/CheckboxList/index.js | Removed PropTypes from the component. |
| src/CheckableList/index.js | Refactored component props and removed PropTypes definitions. |
| src/AnalyzerLayerList/index.js | Removed PropTypes for the map prop and updated default parameters. |
| src/AlertsModal/index.js | Removed PropTypes from the AlertsModal component. |
| src/AddToPatrolModal/index.js | Removed PropTypes from the component. |
| src/AddToIncidentModal/index.js | Removed PropTypes from the component. |
| src/AddNoteButton/index.js | Removed defaultProps and PropTypes definitions, setting defaults inline. |
| src/AddItemButton/AddItemModal/index.js | Removed PropTypes from the modal component. |
| src/AddItemButton/AddItemModal/TypesList/index.js | Removed forwardRef and PropTypes from the TypesList component. |
| src/AddItemButton/AddItemModal/AddPatrolTab/index.js | Removed PropTypes from the component. |
| src/AddAttachmentButton/index.js | Removed defaultProps and PropTypes definitions from the component. |
Files not reviewed (1)
- package.json: Language not supported
| const Input = ({ | ||
| className, | ||
| date, | ||
| isMonthCalendarOpen, | ||
| onClick, | ||
| // We ignore the react-datepicker internals for the keydown event and just handle the Escape ourselves. | ||
| onKeyDown: _onKeyDown, | ||
| ref, | ||
| setIsMonthCalendarOpen, | ||
| ...otherProps | ||
| }, ref) => { | ||
| }) => { |
There was a problem hiding this comment.
The 'ref' prop is still being accepted even though forwardRef has been removed, which may lead to confusion about ref usage. Consider removing or renaming this prop if it’s no longer used for ref forwarding.
Copilot uses AI. Check for mistakes.
| @@ -12,7 +12,7 @@ import GpsInput from '../../GpsInput'; | |||
|
|
|||
| import * as styles from './styles.module.scss'; | |||
|
|
|||
| const MenuPopover = ({ buttonRef, className, onClose, ...otherProps }, ref) => { | |||
| const MenuPopover = ({ buttonRef, className, onClose, ref, ...otherProps }) => { | |||
There was a problem hiding this comment.
Removing forwardRef changes how refs are handled. Verify that consumers of MenuPopover do not rely on ref forwarding and adjust the usage if necessary.
| const MenuPopover = ({ buttonRef, className, onClose, ref, ...otherProps }) => { | |
| const MenuPopover = React.forwardRef(({ buttonRef, className, onClose, ...otherProps }, ref) => { |
Copilot uses AI. Check for mistakes.
| @@ -17,7 +16,7 @@ const CategoryList = ({ category, onClickType, showTitle }) => <div> | |||
| </ul> | |||
| </div>; | |||
|
|
|||
| const TypesList = ({ filterText, onClickType, typesByCategory }, ref) => { | |||
| const TypesList = ({ filterText, onClickType, ref, typesByCategory }) => { | |||
There was a problem hiding this comment.
Avoid using 'ref' as a regular prop name since it conflicts with the reserved React ref. Consider renaming it to a more descriptive term if ref forwarding is not required.
| const TypesList = ({ filterText, onClickType, ref, typesByCategory }) => { | |
| const TypesList = ({ filterText, onClickType, containerRef, typesByCategory }) => { |
Copilot uses AI. Check for mistakes.
|
@gaboDev RT is working properly now. Feel free to give it another pass! Copilot comments don't seem very accurate 🤔 It doesn't like the changes to the |
What does this PR do?
Upgrade React to version 19 and all the React libraries we use. As part of the migration, all prop types and
forwardRefare removed.Relevant link(s)
Where / how to start reviewing (optional)
All the changes are related to the upgrade of React and the React libraries we use.
Any background context you want to provide(if applicable)
This is the third sequential PR to move away from CRA and Webpack. The next step is upgrading Jest and finish with all the dependency updates so we can try to move to Vite ecosystem.